Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate job config settings to publishers section for updating from 1.8 to 2.0 #170

Merged
merged 3 commits into from
Mar 14, 2016
Merged

Migrate job config settings to publishers section for updating from 1.8 to 2.0 #170

merged 3 commits into from
Mar 14, 2016

Conversation

OwensCode
Copy link
Contributor

This is a fix for #106 and includes:

  • Everything from PR 158 from @nevermindr
  • Fixes for issues that were uncovered during testing of PR 158
  • Unit tests for the migration under various scenarios

serg3k and others added 3 commits December 31, 2015 11:52
@serg3k @OwensCode
migrating settings from job config.properties to publishers section
89c9a83 
testing T.B.D
@serg3k @OwensCode
logged IOException
af8e4eb
@OwensCode
[JENKINS-106] Fixes for settings migration to 2.0
a18c6f5 
- Unit tests for configuration migration

- Fix for premature exit of migration because a job without the old
  settings is encountered

- Fix for jobs that have Slack job properties because they were edited
  after the was plugin installed, but do not send notifications

- Fix to remove the Slack job properties from the configuration, even if
  there is no notifier
@jenkinsadmin
Copy link
Member

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@OwensCode OwensCode mentioned this pull request Feb 2, 2016
Closed
@kmadel kmadel added this to the slack-2.0 milestone Mar 6, 2016
@kmadel
Copy link
Contributor

kmadel commented Mar 7, 2016

I would like to merge this as part of the 2.0 release but need at least one more code review - anyone?

@kmadel
Copy link
Contributor

kmadel commented Mar 8, 2016

Anyone else able/willing to test and review this PR?

@kmadel
Copy link
Contributor

kmadel commented Mar 14, 2016

Tested locally against Jenkins core 1.642.2
Upgraded 1.8.1 to 2.0-snapshot with this PR, configuration was successfully migrated.

kmadel added a commit that referenced this pull request Mar 14, 2016
@kmadel
Merge pull request #170 from OwensCode/issue-106-settings-migration
2ab6210 
Migrate job config settings to publishers section for updating from 1.8 to 2.0
@kmadel kmadel merged commit 2ab6210 into jenkinsci:master Mar 14, 2016
@OwensCode OwensCode deleted the issue-106-settings-migration branch March 15, 2016 15:00
@kmadel
Copy link
Contributor

kmadel commented Mar 16, 2016

@OwensCode It appears that the SlackProperty inner-class not having a descriptor is a fatal error in Jenkins 1.652 - see #188

Would you be able to update with something similar to this from https://github.com/jenkinsci/hipchat-plugin/blob/master/src/main/java/jenkins/plugins/hipchat/HipChatNotifier.java:

        @Extension
        public static final class DescriptorImpl extends JobPropertyDescriptor {
            public String getDisplayName() {
                return "HipChat Notifications";
            }

            @Override
            public boolean isApplicable(Class<? extends Job> jobType) {
                return true;
            }

            @Override
            public HipChatJobProperty newInstance(StaplerRequest sr, JSONObject formData) throws hudson.model.Descriptor.FormException {
                return new HipChatJobProperty(sr.getParameter("hipChatProjectRoom"),
                        sr.getParameter("hipChatStartNotification") != null,
                        sr.getParameter("hipChatNotifyAborted") != null,
                        sr.getParameter("hipChatNotifyFailure") != null,
                        sr.getParameter("hipChatNotifyNotBuilt") != null,
                        sr.getParameter("hipChatNotifySuccess") != null,
                        sr.getParameter("hipChatNotifyUnstable") != null,
                        sr.getParameter("hipChatNotifyBackToNormal") != null);
            }
        }

@OwensCode
Copy link
Contributor Author

I'll look into it.

@frostmar frostmar mentioned this pull request Jun 24, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
slack-2.0
Development

Successfully merging this pull request may close these issues.
5 participants
@OwensCode @jenkinsadmin @kmadel @samrocketman @serg3k